Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nuke EKS clusters #43

Merged
merged 6 commits into from
Feb 21, 2019
Merged

Nuke EKS clusters #43

merged 6 commits into from
Feb 21, 2019

Conversation

yorinasub17
Copy link
Contributor

Given how expensive EKS is (one cluster = $0.20/hr), and how frequent the tests fail right now as I actively develop, I am afraid I might forget to clean up one of these so decided to go ahead and implement the nuking.

Note that this required upgrading aws-sdk-go. I tried the latest and there were issues, so downgraded to what we have in terratest.

Copy link
Member

@brikis98 brikis98 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great, thanks Yori. BTW, don't forget to update the README!

aws/eks_test.go Outdated
})
if err != nil {
require.Fail(t, errors.WithStackTrace(err).Error())
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use require.NoError here and below on similar if/Fail combos.

}
rand.Seed(time.Now().UnixNano())
randIndex := rand.Intn(len(supportedRegions))
return supportedRegions[randIndex]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a terratest GetRandomRegion helper that takes in a whitelist and/or blacklist of regions you can use...

)

// getAllEksClusters returns a list of strings of EKS Cluster Names that uniquely identify each cluster.
func getAllEksClusters(awsSession *session.Session, excludeAfter time.Time) ([]*string, error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't you need to call this EKS code from somewhere? I think there's a struct of some sort you need to add and then some extra API calls in the main CLI app...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup... I can't believe I missed that...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was so tied up in the tests and it was getting late that I got a bit too trigger happy I guess.

@yorinasub17
Copy link
Contributor Author

One downside of this: the tests now have an extra 20 minutes because launching and tearing down EKS clusters take that long...

@brikis98
Copy link
Member

One downside of this: the tests now have an extra 20 minutes because launching and tearing down EKS clusters take that long...

Does it run in parallel with the other tests so the whole build takes 20 min? Or is it sequential?

@yorinasub17
Copy link
Contributor Author

It runs parallel, so the whole build takes 20 minutes. For comparison, it was ~5 mins before.

@brikis98
Copy link
Member

brikis98 commented Dec 1, 2018

Understood. Not much we can do, TBH. Eventually, we'll want to nuke RDS, Elasticsearch, and other slow stuff too, so the build will only get slower. Such is DevOps.

@rfvermut
Copy link

rfvermut commented Jan 4, 2019

Can anything be done to finish this?

@yorinasub17
Copy link
Contributor Author

@rfvermut Thanks for the inquiry! I plan on getting back to this sometime later this month or early next month. The main issue right now is that our CI build has gotten unstable with the introduction of the new tests here so need to get a handle on that.

@autero1
Copy link
Contributor

autero1 commented Feb 21, 2019

We don't seem to have verbose output in the tests. Are there any particular tests that keep failing?

@tonerdo
Copy link
Contributor

tonerdo commented Feb 21, 2019

@autero1 see #50

@yorinasub17
Copy link
Contributor Author

Thanks @tonerdo for the test fixes! I took a quick scan and I believe I addressed all the comments, so I am going to go ahead and merge this and release!

@yorinasub17 yorinasub17 merged commit bead08e into master Feb 21, 2019
@yorinasub17 yorinasub17 deleted the yori-nuke-eks branch February 21, 2019 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants